Skip to content

TYP: indexes/base.py #39897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 21, 2021
Merged

TYP: indexes/base.py #39897

merged 2 commits into from
Feb 21, 2021

Conversation

topper-123
Copy link
Contributor

Add types to pandas/core/indexes/base.py.

@@ -571,11 +580,11 @@ def _simple_new(cls, values, name: Hashable = None):
return result

@cache_readonly
def _constructor(self):
def _constructor(self: _IndexT) -> Type[_IndexT]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it matter that a couple of subclasses behave differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive typed the subclass methods, I dont think this is a problem.

@@ -3663,7 +3672,7 @@ def _invalid_indexer(self, form: str_t, key) -> TypeError:
# Reindex Methods

@final
def _can_reindex(self, indexer):
def _can_reindex(self, indexer) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misleading name? id expect it to return bool

@@ -3403,7 +3412,7 @@ def _check_indexing_method(self, method):

raise ValueError("Invalid fill method")

def _convert_tolerance(self, tolerance, target):
def _convert_tolerance(self, tolerance, target) -> np.ndarray:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think target at this point is an Index (worth double-checking)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can also be an ndarray.

@@ -2611,7 +2620,7 @@ def drop_duplicates(self, keep="first"):

return super().drop_duplicates(keep=keep)

def duplicated(self, keep="first"):
def duplicated(self, keep="first") -> np.ndarray:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep is str?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can accept a str or a literal False. I've set the type to Union[str, bool].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a different PR, bit I don't like the bool type, that can not accept True. I'd like to deprecate the bool type and accept only string parameter options, e.g. "none" instead of False. Any comment on that?

@@ -2560,7 +2569,7 @@ def unique(self, level=None):
return self._shallow_copy(result)

@final
def drop_duplicates(self, keep="first"):
def drop_duplicates(self: _IndexT, keep="first") -> _IndexT:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep is str? (maybe even define literal in pd._typing)

"""
Return unique values in the index.

Unique values are returned in order of appearance, this does NOT sort.

Parameters
----------
level : int or str, optional, default None
level : int or hashable, optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"hashable" seems weird here. may parenthetically "level name"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried adding a text below the types.

@@ -1243,7 +1252,7 @@ def to_flat_index(self):
"""
return self

def to_series(self, index=None, name=None):
def to_series(self, index=None, name=None) -> Series:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name -> Hashable?

@jreback jreback added the Typing type annotations, mypy/pyright type checking label Feb 19, 2021
@topper-123
Copy link
Contributor Author

Updated.

@jreback jreback merged commit 479a7f6 into master Feb 21, 2021
@jreback
Copy link
Contributor

jreback commented Feb 21, 2021

thanks @topper-123

@topper-123 topper-123 added this to the 1.3 milestone Feb 21, 2021
@topper-123 topper-123 deleted the type_index branch February 21, 2021 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants